Change absolute paths to relative paths in docs #4698

Atish-iaf · 2023-03-11T18:02:32Z

No description provided.

Atish-iaf · 2023-03-11T18:04:45Z

/skip-all

luolanzone · 2023-03-14T14:23:07Z

docs/aks-installation.md

@@ -73,7 +73,7 @@ you can deploy Antrea as follows:
    ```

    To deploy the latest version of Antrea (built from the main branch), use the
-checked-in [deployment yaml](/build/yamls/antrea-aks.yml):
+checked-in [deployment yaml](https://github.com/antrea-io/antrea/blob/main/build/yamls/antrea-aks.yml):


I believe @tnqn mentioned that the root cause is we should use relative path in the issue comment.
you should update it as following one.

Suggested change

checked-in [deployment yaml](https://github.com/antrea-io/antrea/blob/main/build/yamls/antrea-aks.yml):

checked-in [deployment yaml](./build/yamls/antrea-aks.yml):

This wouldn't work because it expands to wrong path and searches for the file in antrea website repo.
Actually any relative path wouldn't work here because these files (mostly yaml) are not present in antrea website github repo.

If relative path has to work , the folder "build/yamls/" needs to be created on https://antrea.io and then each file needs to be available there, otherwise will have to use absolute GitHub path

I doubt this is the right way to fix the issue. If you change it to a link, then when you have any changes on a local file, it will jump to a website link which point to a main branch file. I don't think this is what we expected.
Let's see what's @antoninbas and @tnqn 's comments.

All the docs should use relative paths. The link should be ../build/yamls/antrea-aks.yml here

The right way to fix it is in the scripts which generate the website source (they should do link substitution when applicable: ../build/yamls/antrea-aks.yml -> https://github.com/antrea-io/antrea/blob/<CORRECT TAG>/build/yamls/antrea-aks.yml)

Thanks, changed to use relative path.

antoninbas

while this change is correct, and needed IMO, it doesn't by itself fix issue #4353, and so adding Fixes #4353 to the commit message is incorrect.

in addition to this change, the https://github.com/antrea-io/website/blob/main/scripts/pkg/update-docs.go logic also needs to be improved to translate links to the build/ directory into absolute http links, as I described in a previous comment.

Atish-iaf · 2023-03-22T07:12:30Z

while this change is correct, and needed IMO, it doesn't by itself fix issue #4353, and so adding Fixes #4353 to the commit message is incorrect.

in addition to this change, the https://github.com/antrea-io/website/blob/main/scripts/pkg/update-docs.go logic also needs to be improved to translate links to the build/ directory into absolute http links, as I described in a previous comment.

Hi @antoninbas, thanks for your suggestions. I have few questions.

Could you please help me understand why changes made in this PR are needed? We can skip this PR and make changes in website repo to translate existing relative links to appropriate absolute links. Also, the existing relative links works fine when inside local antrea repo, so why to add .. as prefix in existing relative links. Both will work same when inside local antrea repo.
At few places in documentation (https://github.com/antrea-io/antrea/blob/main/docs/aks-installation.md#:~:text=To%20deploy%20the%20latest%20version%20of%20Antrea%20(built%20from%20the%20main%20branch)%2C%20use%20the%20checked%2Din%20deployment%20yaml%3A) link should be to the latest antrea.yml, so at these places can we put absolute link to antrea.yml from main branch of antrea github repo? (Here, I am saying not to translate)

antoninbas · 2023-03-22T17:37:52Z

It is not strictly needed for these links to the build/ directory (since we have to transform them manually anyway). However, as a rule, the website scripts don't render absolute documentation links correctly (by absolute, I mean like /docs/foo.md, and not URLs) and it is safer to always stick to relative links (docs/foo.md, ../docs/foo.md). Ideally we would have a lint rule to prevent absolute markdown links (i.e., links starting with a /). It should be possible for us to add a custom rule to markdownlint.
No, it is not correct to always point to the main version. The antrea.io website includes a different version of the documentation for each recent release (check https://antrea.io/docs/v1.10.0/). When visiting the documentation for an older release, links should point to the correct version of the manifests (and not point to main). This is why I described the "correct link substitution" in Change absolute paths to relative paths in docs #4698 (comment).

Signed-off-by: Kumar Atish <atish.iaf@gmail.com>

antoninbas · 2023-03-23T17:34:57Z

/skip-all

Signed-off-by: Kumar Atish <atish.iaf@gmail.com>

Atish-iaf requested a review from luolanzone March 14, 2023 11:37

luolanzone reviewed Mar 14, 2023

View reviewed changes

Atish-iaf force-pushed the FixLinksInDocuments branch 2 times, most recently from e238b07 to 464d7f6 Compare March 17, 2023 10:53

Atish-iaf requested a review from antoninbas March 17, 2023 10:56

antoninbas reviewed Mar 17, 2023

View reviewed changes

Change absolute paths to relative paths in docs

bd07c67

Signed-off-by: Kumar Atish <atish.iaf@gmail.com>

Atish-iaf force-pushed the FixLinksInDocuments branch from 464d7f6 to bd07c67 Compare March 23, 2023 07:32

Atish-iaf changed the title ~~Fix wrong links in docs~~ Change absolute paths to relative paths in docs Mar 23, 2023

Atish-iaf requested a review from antoninbas March 23, 2023 11:31

jianjuns approved these changes Mar 23, 2023

View reviewed changes

antoninbas approved these changes Mar 23, 2023

View reviewed changes

antoninbas merged commit 605a9d6 into antrea-io:main Mar 23, 2023

jainpulkit22 pushed a commit to urharshitha/antrea that referenced this pull request Apr 28, 2023

Change absolute paths to relative paths in docs (antrea-io#4698)

6cd0f0e

Signed-off-by: Kumar Atish <atish.iaf@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change absolute paths to relative paths in docs #4698

Change absolute paths to relative paths in docs #4698

Atish-iaf commented Mar 11, 2023 •

edited

Loading

Atish-iaf commented Mar 11, 2023

luolanzone Mar 14, 2023 •

edited

Loading

Atish-iaf Mar 14, 2023

rajnkamr Mar 15, 2023

luolanzone Mar 17, 2023

antoninbas Mar 17, 2023

Atish-iaf Mar 17, 2023

antoninbas left a comment •

edited

Loading

Atish-iaf commented Mar 22, 2023

antoninbas commented Mar 22, 2023

antoninbas commented Mar 23, 2023

	checked-in [deployment yaml](https://github.com/antrea-io/antrea/blob/main/build/yamls/antrea-aks.yml):
	checked-in [deployment yaml](./build/yamls/antrea-aks.yml):

Change absolute paths to relative paths in docs #4698

Change absolute paths to relative paths in docs #4698

Conversation

Atish-iaf commented Mar 11, 2023 • edited Loading

Atish-iaf commented Mar 11, 2023

luolanzone Mar 14, 2023 • edited Loading

Choose a reason for hiding this comment

Atish-iaf Mar 14, 2023

Choose a reason for hiding this comment

rajnkamr Mar 15, 2023

Choose a reason for hiding this comment

luolanzone Mar 17, 2023

Choose a reason for hiding this comment

antoninbas Mar 17, 2023

Choose a reason for hiding this comment

Atish-iaf Mar 17, 2023

Choose a reason for hiding this comment

antoninbas left a comment • edited Loading

Choose a reason for hiding this comment

Atish-iaf commented Mar 22, 2023

antoninbas commented Mar 22, 2023

antoninbas commented Mar 23, 2023

Atish-iaf commented Mar 11, 2023 •

edited

Loading

luolanzone Mar 14, 2023 •

edited

Loading

antoninbas left a comment •

edited

Loading